libexpr: derivationOf primop #14956
libexpr: derivationOf primop #14956
Conversation
github-actions
bot
added
the
with-tests
|
This should reject nonsensical inputs, just like my Nixpkgs implementation would.
Overly lenient functions produce garbage inputs to other functions, which then produce garbage errors, and we can't make functions less lenient without breaking user expressions, so this really has to be done with care and a bunch of "adversarial" tests. |
JustAGuyTryingHisBest
force-pushed
the
justaguytryinghisbest/derivationOfPrimop
branch
from
0fff477 to
210b7fc
Compare
github-actions
bot
added
new-cli
JustAGuyTryingHisBest
force-pushed
the
justaguytryinghisbest/derivationOfPrimop
branch
from
210b7fc to
08f9ac6
Compare
|
Awesome. Could you make sure the returned string has the correct context? |
|
@roberth I appreciate your patience and working through this change with me!
Done. Does this seem like the right context?
Done. Do you see any additional tests that might be beneficial?
see nix-repl> builtins.derivationOf "/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-test"
error:
… while calling the 'derivationOf' builtin
at «string»:1:1:
1| builtins.derivationOf "/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-test"
| ^
error: '/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-test' has no derivation in its context
I would opt to open an issue against the ca-derivation milestone. |
| system = "x86_64-linux"; | ||
| }; | ||
| # Create a string with multiple context items by concatenating paths from different derivations | ||
| multipleContexts = "${drv1.outPath} ${drv2.outPath}"; |
There was a problem hiding this comment.
Slightly more interesting test case this way, triggering the multiple contexts error instead, iiuc.
| multipleContexts = "${drv1.outPath} ${drv2.outPath}"; | |
| multipleContexts = "${drv1.outPath}${drv2.outPath}"; |
| system = "x86_64-linux"; | ||
| }; | ||
| in | ||
| builtins.derivationOf drv.outPath == drv.drvPath |
There was a problem hiding this comment.
Extra test case?
I think this works, through outPath string coercion.
| builtins.derivationOf drv.outPath == drv.drvPath | |
| builtins.derivationOf drv == drv.drvPath |
It might look wrong on the face of it, because drv feels the same as drvPath, but this is actually good.
I'd much prefer to talk about "packages" despite type = "derivation"; on these things; that's just historical.
derivationOf pkg == pkg.drvPath already looks better, and we might as well name it that way in the test.
Packages more closely reflects how we reason about these things. They're all about the outputs, and the derivation (recipe) is just an implementation detail.
String context and outPath string coercion are designed to facilitate that way of thinking.
I've also yapped about this in #6507, and this builtin helps with that direction, by making drvPath largely unnecessary. (And the equality in this test shows the redundancy!)
There was a problem hiding this comment.
Makes sense. I appreciate the context. I went ahead and added these changes.
3 participants
Motivation
Taken directly from the related issue - This function improves RFC 92 and multi-derivation package support, by allowing the derivation path to be accessed for any output, whose derivations may be distinct, rather than only exposing a single drvPath.
Context
Resolves #10120
Translates solution from NixOS/nixpkgs#281536 to CppNix. Thank you to @roberth for the initial implementation.
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.